Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

refactor(rome_analyze): automatically derive the rule category in declare_rule! #3321

Merged
merged 5 commits into from
Oct 5, 2022

Conversation

leops
Copy link
Contributor

@leops leops commented Oct 4, 2022

Summary

This PR modifies the declare_rule! macro to automatically assign the correct RuleCategory to the metadata of a rule, instead of having to explicitly specify it as an associated constant on the Rule trait. This change relies on the specific module structure of the language analyzer implementations, specifically that rules are implemented in nested modules following the <category>::<group>::<rule> pattern in order to access information about the group of the rule using super, and information about the category using super::super.

I also decided to try and go one step further with this change by actually tying the rule to their group, and the groups to their category at the type system level (using an associated type on a trait). This means the category / group / rule hierarchy is now cyclic as each of these item knows both about it's parent and / or children, but that doesn't pose any problem in practice since this is only type-level metadata that gets resolved lazily by the Rust compiler as the generic code gets monomorphized. It does allow making a number of generic functions simple however, since they now only need to be generic on the type of a lint rule and can access the type of the group using the Group associated type, instead of being provided with the group as a second generic type.

Test Plan

This is mostly type-level only change, it should only have minimal impact on the how the code works. One notable exception is a change to the filtering code (filtering of categories and groups now happens earlier in the registry building code), but this behavior is largely covered by the existing analyzer tests.

@leops leops requested a review from xunilrj as a code owner October 4, 2022 14:30
@leops leops requested a review from a team October 4, 2022 14:30
@netlify
Copy link

netlify bot commented Oct 4, 2022

Deploy Preview for rometools canceled.

Name Link
🔨 Latest commit caf979c
🔍 Latest deploy log https://app.netlify.com/sites/rometools/deploys/633c575862b3c80008492137

@leops leops had a problem deploying to netlify-playground October 4, 2022 14:30 Failure
@github-actions
Copy link

github-actions bot commented Oct 4, 2022

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45879 45879 0
Passed 44939 44939 0
Failed 940 940 0
Panics 0 0 0
Coverage 97.95% 97.95% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 39 39 0
Passed 36 36 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.31% 92.31% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 5946 5946 0
Passed 1621 1621 0
Failed 4325 4325 0
Panics 0 0 0
Coverage 27.26% 27.26% 0.00%

ts/babel

Test result main count This PR count Difference
Total 588 588 0
Passed 519 519 0
Failed 69 69 0
Panics 0 0 0
Coverage 88.27% 88.27% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16257 16257 0
Passed 12395 12395 0
Failed 3862 3862 0
Panics 0 0 0
Coverage 76.24% 76.24% 0.00%

@leops leops force-pushed the refactor/rule-categories branch from 728d198 to 8b178af Compare October 4, 2022 15:22
@leops leops temporarily deployed to netlify-playground October 4, 2022 15:22 Inactive
@github-actions
Copy link

github-actions bot commented Oct 4, 2022

@leops leops requested a review from ematipico as a code owner October 4, 2022 15:47
@leops leops temporarily deployed to netlify-playground October 4, 2022 15:47 Inactive
ematipico
ematipico approved these changes Oct 4, 2022
crates/rome_analyze/src/rule.rs Outdated Show resolved Hide resolved
crates/rome_analyze/CONTRIBUTING.md Show resolved Hide resolved
Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
@leops leops temporarily deployed to netlify-playground October 4, 2022 15:55 Inactive
@leops leops merged commit a0a520f into main Oct 5, 2022
@leops leops deleted the refactor/rule-categories branch October 5, 2022 07:43
@leops leops added A-Diagnostic Area: errors and diagnostics A-Linter Area: linter labels Oct 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Diagnostic Area: errors and diagnostics A-Linter Area: linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants